Skip to content

[Fix] Upload path_in_repo Normalization - #69

Merged
wangxingjun778 merged 18 commits into
mainfrom
feat/latest_openapi
Sep 20, 2026
Merged

wangxingjun778 merged 18 commits into
mainfrom
feat/latest_openapi

Conversation

@wangxingjun778

Copy link
Copy Markdown
Member

Fix

  • Uploads with path_in_repo="." no longer fail. A . destination (e.g. ms upload REPO DIR .) was written into every commit action as a literal ./ path prefix, which the Hub rejected with E3021 "invalid commit action" on every commit — aborting the run after 3 consecutive failed batches. . now correctly resolves to the repository root.

Enhance

  • Robust destination-path handling. path_in_repo is now normalized consistently across upload_folder and upload_file: ., ./, /, and empty all map to the repo root; redundant separators and ./.. segments are collapsed; and paths that escape the repository root are rejected upfront.

…ssion tiers

Audited the live OpenAPI document (1.1.0+master.20260813) against the client:
of the 23 operations under the MCP and Studios tags, 8 were unimplemented and
several implemented ones had drifted from the spec. Closes the gap and adds a
guard so the next drift fails the suite instead of going unnoticed.

Feature
- Studios: list_studios, list_studio_hardware / _base_images / _sdk_versions,
  and the four plaintext-variable operations; exposed as `ms-hub studio list`,
  `studio variable`, `studio hardware|base-images|sdk-versions`, and
  `ms-hub list --repo-type studio`.
- MCP: HubApi.list_operational_mcp_servers() plus `ms-hub mcp list --hosted`,
  which also unblocks the umbrella SDK delegating its own MCPApi here.
- Studio visibility gains `protected` (app public, code hidden), previously
  inexpressible because visibility was folded into a `private` bool.
- RepoInfo now carries the Studio runtime fields (sdk_type, sdk_version,
  base_image, hardware, mcp_support, runtime) instead of discarding them.
- Token permission tiers: TokenScope annotations, a read-scoped token can log in
  (warning that writes need a higher tier), and a rejected write names the tier
  it requires.

Fix
- The compat layer forwarded `token` / `endpoint` into request bodies, so
  `HubApi().update_studio_settings(..., token=...)` serialised the caller's API
  token into the PATCH body. All Studio shims now split control kwargs from
  business fields.
- `HubApi().get_studio_logs(..., token=...)` raised TypeError and could never
  succeed; a per-call `token=` was silently ignored on every Studio shim.
- `cover_image` was renamed to `coverImage` on the way out and dropped by the
  server, so Studio cover images never applied.
- 403 now separates insufficient permission from an exhausted quota (new
  QuotaExceededError, deliberately not retryable); 409 maps to
  AlreadyExistsError; the OpenAPI string error codes are recognised at all.
- `get_studio` demanded a token for spaces the spec says are public.
- `studio logs` read a `total` field the response never had, so its pagination
  footer never printed.

Enhance
- MCP discovery sends PUT first -- the only verb the spec defines -- and
  remembers which verb a deployment serves, so the loser is tried at most once.
- Listing a Studio owner asks for every status: the endpoint keeps filtering to
  running spaces even with an owner set, so "list my spaces" returned nothing.

Documentation
- README: new Studio/MCP command surface, a Token permission levels section, and
  the SDK method reference.

Testing
- Vendored the spec under tests/data/ and check it against OPERATION_REGISTRY.
- 723 unit tests (881 in mock mode) and 28 integration tests against the live
  service.
The News list had no entry for 0.3.0 or 0.3.1, so it jumped straight from 0.4.0
to 0.2.0 and left the release that took over all four console scripts
undocumented.

- Adds v0.3.0 (unified console scripts, repo/files truncation mitigation,
  get_current_username, agent visibility) and v0.3.1 (upload env var rename with
  deprecation shims, logout wiring, download lock paths, UserInfo and legacy
  compat field mapping), both reconstructed from the commits in each range.
- Condenses v0.4.0 from eleven bullets to nine by merging entries that share a
  root cause: the three compat kwargs defects are one leak, and the two MCP/
  Studio request-shape improvements are one behaviour change.
- Folds v0.2.0 and everything older into the "Older releases" block, so only the
  three current releases are visible by default.
- v0.4.0: fold the second Feature bullet into the OpenAPI-coverage one, join the
  cover_image leak into the compat-kwarg fix, and merge the two small studio
  fixes into the error-handling bullet — nine bullets become six, one per
  category except Feature.
- v0.3.0/v0.3.1: trim wordy phrasing and group Feature items first.
…server limits

Uploading a large dataset of small files was dominated by commit count and
commit body size, not by bandwidth. Non-LFS files travel base64-inlined inside
the commit request, so a 60k-file corpus put gigabytes through the API servers
and forced tiny batches, multiplying commits against the per-repo hourly commit
budget.

Upload path:
- LFS threshold becomes byte-level (MODELSCOPE_UPLOAD_LFS_FORCE_THRESHOLD,
  accepts 32KiB/512K/1MiB/0); the MB-only name stays as a deprecated alias.
  Default is unchanged at 1MiB.
- UPLOAD_INLINE_METADATA_PATHS keeps README.md, .gitattributes and the
  configuration files inline whatever the threshold, so lowering it can never
  turn a file the Hub parses into an LFS pointer.
- Commit batches are planned on two axes: operation count and inlined content
  volume (UPLOAD_COMMIT_MAX_INLINE_BYTES, 8MiB). Adaptive sizing no longer
  silently overrides an explicit cap, and both are clamped to the server's
  COMMIT_MAX_ACTIONS_PER_REQUEST ceiling, which is rejected rather than trimmed.
- delete_files splits at the same ceiling instead of sending every path at once.
- All LFS candidates are hashed up front and pre-signed in parallel groups,
  replacing one round trip per file. A group that fails degrades to per-file
  negotiation; an oid absent from a *failed* group is never read as "already
  exists", which would skip the transfer and commit a dangling pointer.

Transport and resilience:
- Both HTTP sessions size their pool from API_CONNECTION_POOL_MAXSIZE (32).
  urllib3's default of 10 discarded connections under the upload worker count
  and paid for a fresh TLS handshake on each.
- Commit retries honor a server Retry-After, budgeted apart from the
  transient-error allowance, plus an opt-in per-hour commit governor shared by
  every commit path. upload_file now uses that path too.
- A tree listing denied with 403 after the credential has been proven by an
  earlier successful read is retried; a large listing spans hundreds of requests,
  so a per-request denial rate otherwise guarantees whole-listing failure.

Observability:
- upload_folder takes progress_callback and emits per-batch commit events plus
  throttled wire-level upload_progress events carrying byte counts, so a
  consumer can build a rate and an ETA instead of watching a silent run.
- upload_folder takes tracker_path, so resume state can live outside a staging
  tree that gets discarded between runs.
- Invalid environment values now warn and name the fallback instead of reverting
  silently.

Verified end to end against the Hub: 25000 files / 5.00 GiB and 10000 files /
2.00 GiB, both 0 failures, sha256 spot-checks identical, commit count 254 -> 49
and inlined commit volume 1.96GiB -> 55MiB on the 5 GiB corpus.
…overed work

Batch pre-signing asks the server about every distinct oid before any upload
starts, so the server cannot answer "already stored" for a duplicate -- it is not
stored yet. Confirmed against the Hub: pre-signing the same fresh oid twice
returns two upload URLs, and only after the blob lands does the endpoint report
it as existing. Every occurrence of identical content therefore PUT the same
bytes, which the previous per-file pre-sign path had avoided by accident of
timing.

Owner election, decided before any worker starts:
- The first file holding an oid owns its transfer; the rest carry a marker and
  skip the PUT. `files_to_upload` is in ascending file index and a batch owns a
  contiguous ascending range, so an owner always lands in a batch no later than
  its copies. Batches commit in order and each waits for its own files, so a
  copy's blob is guaranteed present by the time its batch commits -- no locks, no
  waiting, and no way for the worker pool to block on itself.
- Content the server already holds needs no owner at all.
- If an owner's upload fails, its copies are withheld from the commit and pushed
  to the retry queue rather than published as pointers to a blob that was never
  stored.
- Inline files are untouched: their content travels in the commit body, so there
  is no transfer to skip.

Recovery accounting:
- The ReAct and simple retry paths now report their uploads and commits. They
  previously committed silently, so any rescued volume never reached the metrics.
  An 8 GiB run that lost one 512-file batch to a rejected commit finished with
  all 40000 files on the Hub but 91 MB missing from done_bytes.

Verified against the Hub with 40000 files / 8.00 GiB (37543 distinct contents,
largest duplicate group 1272):
- predicted saving 51,369,983 bytes; the run transferred 8,538,564,609 of
  8,589,934,592 committed bytes -- short by exactly 51,369,983.
- 12 of the 385 files that skipped their own PUT were downloaded and hashed:
  all matched, so no pointer was left dangling.
- listing check: 40000 remote blobs, 0 missing, 0 extra, 0 size mismatches.
- a fresh-state re-upload moved 64.81 MiB instead of 8 GiB and reported
  done_bytes exactly, confirming the recovery accounting fix.
path_in_repo="." (e.g. `ms upload REPO DIR .`) became a literal "./" prefix on every file, which the Hub rejects with E3021 "invalid commit action" on every commit, aborting the upload after 3 consecutive failed batches. Collapse ".", "./", "" and "/" to the repo root, normalize separators and ".", ".." segments, and refuse paths that escape the root. Applied in both upload_folder and upload_file; added regression tests. Verified end-to-end with a real 14131-file / 2.4GB dataset upload using path_in_repo=".": all files committed, 0 E3021, 0 aborts, server paths carry no "./" prefix.
@wangxingjun778
wangxingjun778 merged commit 3add11a into main Sep 20, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant